Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Folder structure and placeholders for defining event schemas (for RUM events as a start) #2933

Closed
wants to merge 7 commits into from

Conversation

ramthi
Copy link

@ramthi ramthi commented Nov 9, 2022

This PR is to approve the folder structure for documenting various event schemas. The RUM SIG had been working on defining schemas for various RUM events.

The WIP documents are here:
https://docs.google.com/document/d/1_5MtDy_8uovK3cQEmwrVmFz0ApLQvQGsYX4ME9Kl3pg/edit#heading=h.nbj5nuoe6d1n
https://docs.google.com/spreadsheets/d/1WEIhp7EX6nOg6eJPg6V5OrIPE8EE-oRD4KoiK_6XBF0/edit#gid=267654663

The folder structure follows ../logs/semantic_conventions//.md format.

For the browser domain there are standard set of attributes/fields. We want to define these in the common.md file to avoid redundancy. The common. md is referenced in each of the event schemas in the attributes table.

The attributes listed in these event schemas will be nested attributes under the event.data field. Pl. refer to this (PR)[https://pull/2926] for the proposal to add event.data nested attribute.

@MSNev
Copy link
Contributor

MSNev commented Nov 9, 2022

tagging myself

@ramthi
Copy link
Author

ramthi commented Nov 9, 2022

@scheler, @martinkuba, @MSNev Pl. review.

@tigrannajaryan
Copy link
Member

Do we want this in the spec? Why isn't it defined in the JS SIG's documentation? Is this applicable to more than one language?

@ramthi
Copy link
Author

ramthi commented Nov 10, 2022

Do we want this in the spec? Why isn't it defined in the JS SIG's documentation? Is this applicable to more than one language?

Yes, that is what the RUM SIG desires. IMO, this is schema, and putting it inside a specific language repo doesn't help discoverability much. Also, as part of RUM we intend to define other events for clients in general. Those schemas would anyway end up in the spec (or another common location) repo. Keeping everything organized here would help drive consistency across client schemas.

@scheler
Copy link
Contributor

scheler commented Nov 10, 2022

Do we want this in the spec? Why isn't it defined in the JS SIG's documentation? Is this applicable to more than one language?

In addition to what Ram said, it will also help us in leveraging the tools built for semantic conventions. These are semantic conventions and we already have browser and mobile specific conventions in the spec repo.

@scheler
Copy link
Contributor

scheler commented Nov 10, 2022

@ramthi a few things -

  1. These semantic conventions have to first go into the top level semantic_conventions folder as yaml files and then use the otel/semconvgen tool to generate the markdown text that goes into specification//semantic_conventions folder.
  2. The attributes you identified as common as Resource level attributes so they go under semantic_conventions/resource/browser.yaml
  3. The others go into either semantic_conventions/trace or semantic_conventions/logs depending on whether we plan to send those attributes in Spans (including Span Events) or Events. For the attributes that can go in either, I think you can put them in one and reference from the other. See logs/log-exception.yaml for an example.

@scheler
Copy link
Contributor

scheler commented Nov 10, 2022

@ramthi Another thing to note is that these are only semantic conventions and not strictly schema defining the shape of Events. That is out of scope for both OpenTelemetry semantic conventions and OpenTelemetry schemas. If we want the schema to define the shape of Events, then that will be a new feature addition to the spec.

@tigrannajaryan
Copy link
Member

I am concerned about the following:

  • (In)ability of spec maintainers to meaningfully review the proposed semantic conventions. The conventions are pretty deep in the browser territory and reviewers here don't necessarily have the required experience.
  • Slower pace of creating such semantic conventions. Everything is slower in spec repo (by design - we require more rigour and scrutiny).
  • Unclear if the proposed conventions need to work across more than just JS. Typically spec is concerned about cross-cutting matters.

@jsuereth what do you think?

@scheler
Copy link
Contributor

scheler commented Nov 10, 2022

@tigrannajaryan RUM is not just browser apps monitoring, it includes mobile apps as well and potentially IoT in the future. The mobile apps use opentelemetry-java and opentelemetry-swift. It will be good to have the conventions for all of these in one place.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 18, 2022
@jack-berg jack-berg removed the Stale label Nov 21, 2022
@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@tigrannajaryan
Copy link
Member

@jsuereth I think this is a topic for Semantic Convention WG to make a call on.

@github-actions github-actions bot removed the Stale label Dec 1, 2022
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 9, 2022
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants